Skip to content

allow scrollable#383

Merged
iceljc merged 1 commit into
SciSharp:mainfrom
iceljc:features/refine-chat-window
Sep 11, 2025
Merged

allow scrollable#383
iceljc merged 1 commit into
SciSharp:mainfrom
iceljc:features/refine-chat-window

Conversation

@iceljc
Copy link
Copy Markdown
Collaborator

@iceljc iceljc commented Sep 11, 2025 •
edited by qodo-code-review Bot
Loading

PR Type

Enhancement

Description

  • Add smooth scrolling to agent component containers

  • Auto-scroll to bottom when adding new items

  • Improve user experience in agent configuration forms

Diagram Walkthrough

flowchart LR
  A["User clicks Add button"] --> B["New item added to list"]
  B --> C["scrollToBottom() function called"]
  C --> D["Container scrolls smoothly to bottom"]
Loading

File Walkthrough
Relevant files
Enhancement
agent-knowledge-base.svelte
Add scrolling to knowledge base component                               

src/routes/page/agent/[agentId]/agent-components/agent-knowledge-base.svelte

  • Add scrollContainer variable for DOM reference
  • Implement scrollToBottom() function with smooth scrolling
  • Bind scroll container to agent utility container div
  • Call scrollToBottom() when adding new knowledge base
 +15/-1   
agent-mcp-tool.svelte
Add scrolling to MCP tool component                                           

src/routes/page/agent/[agentId]/agent-components/agent-mcp-tool.svelte

  • Add scrollContainer variable for DOM reference
  • Implement scrollToBottom() function with smooth scrolling
  • Bind scroll container to agent utility container div
  • Call scrollToBottom() when adding new MCP tool
 +16/-1   
agent-rule.svelte
Add scrolling to agent rule component                                       

src/routes/page/agent/[agentId]/agent-components/agent-rule.svelte

  • Add scrollContainer variable for DOM reference
  • Implement scrollToBottom() function with smooth scrolling
  • Bind scroll container to agent utility container div
  • Call scrollToBottom() when adding new rule
 +15/-1   
agent-utility.svelte
Add scrolling to agent utility component                                 

src/routes/page/agent/[agentId]/agent-components/agent-utility.svelte

  • Add scrollContainer variable for DOM reference
  • Implement scrollToBottom() function with smooth scrolling
  • Bind scroll container to agent utility container div
  • Call scrollToBottom() when adding new utility
 +17/-1   

@iceljc iceljc merged commit 36e078b into SciSharp:main Sep 11, 2025
1 of 2 checks passed
@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Sep 11, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

DOM Flush Timing

Ensure the scroll runs after the DOM has actually rendered the newly added item. Consider using Svelte's tick() (or afterUpdate) instead of setTimeout to guarantee timing and avoid potential race conditions.

 
function scrollToBottom() {
    if (scrollContainer) {
        setTimeout(() => {
            scrollContainer.scrollTo({
                top: scrollContainer.scrollHeight,
                behavior: 'smooth'
            });
        }, 0);
    }
}
Scroll Trigger Placement

The scroll is triggered immediately after updating the list; if subsequent updates occur in handleAgentChange(), the final height may change after the scheduled scroll. Consider awaiting a DOM flush before scrolling, or triggering scroll in a reactive block based on list length.

 
scrollToBottom();
handleAgentChange();
Duplicate Logic

The same scrollToBottom() implementation appears in multiple components. Consider extracting a shared utility or a Svelte action to reduce duplication and keep behavior consistent.

 
function scrollToBottom() {
    if (scrollContainer) {
        setTimeout(() => {
            scrollContainer.scrollTo({
                top: scrollContainer.scrollHeight,
                behavior: 'smooth'
            });
        }, 0);
    }
}

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Sep 11, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Safely schedule and re-check scroll

Re-check the container inside the async callback and use requestAnimationFrame
to run after the next paint. This avoids errors if the component unmounts before
the timeout fires and ensures the DOM has rendered before scrolling.

src/routes/page/agent/[agentId]/agent-components/agent-utility.svelte [276-285]

 function scrollToBottom() {
-    if (scrollContainer) {
-        setTimeout(() => {
-            scrollContainer.scrollTo({
-                top: scrollContainer.scrollHeight,
-                behavior: 'smooth'
-            });
-        }, 0);
-    }
+    // Run after the next paint to ensure DOM/layout is ready
+    requestAnimationFrame(() => {
+        if (!scrollContainer) return;
+        scrollContainer.scrollTo({
+            top: scrollContainer.scrollHeight,
+            behavior: 'smooth'
+        });
+    });
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly proposes using requestAnimationFrame which is better suited for DOM manipulations like scrolling, and it adds a safety check inside the callback, making the code more robust against edge cases.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@iceljc